Skip to content

Comments

Use std::optional for ET feeder node attributes and introduce INVALID_COMM_TYPE in protobuf#3

Open
hxu296 wants to merge 1 commit intoastra-sim:mainfrom
hxu296:et_feeder_node_optional
Open

Use std::optional for ET feeder node attributes and introduce INVALID_COMM_TYPE in protobuf#3
hxu296 wants to merge 1 commit intoastra-sim:mainfrom
hxu296:et_feeder_node_optional

Conversation

@hxu296
Copy link

@hxu296 hxu296 commented Feb 4, 2025

Summary

Updates the handling of optional attributes for ET feeder nodes. Previously, default values for fields like num_ops and tensor_size were uninitialized, potentially returning random values. The changes:

  • Introduce std::optional for fields that may not have a value.
  • Ensure getters return a default value when no value is present.
  • Add an INVALID_COMM_TYPE to the protobuf CollectiveCommType enum to represent a default communication type.

These updates improve the robustness of the code by preventing uninitialized data from being returned.

this->is_cpu_op_ = static_cast<bool>(attr.bool_val());
} else if (attr_name == "num_ops") {
this->num_ops_ = static_cast<uint64_t>(attr.int64_val());
} else if (attr_name == "tensor_loc_") {
Copy link
Collaborator

@JoongunPark JoongunPark Feb 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ: is the attribute name "tensor_loc_" not "tensor_loc"?
What upstream tool uses that name? STG?
I think Chakra converter does not use that attribute name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants